Repository navigation
Conversation
ExtendedEndPoint::to_hostname() printed sin6_port without ntohs(), so endpoint2hostname() of [::1]:8080 gave "localhost:36895" on little endian machines, while endpoint2str() and the IPv4 path print 8080. /connections shows the wrong port for IPv6 peers when -show_hostname_instead_of_ip is on. Generated-by: Claude Code (Claude Fable 5.2) Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Signed-off-by: Dezhen Lu <dezhen.lu@student.uni-tuebingen.de>
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
Fixes incorrect port formatting for IPv6 endpoints in endpoint2hostname() by converting the stored network-order port to host byte order, and adds a regression test to prevent reintroduction.
Changes:
- Convert IPv6 port from network byte order via
ntohs()when formatting hostnames. - Add a unit test covering
endpoint2hostname()port formatting for IPv6 loopback.
| File | Description |
|---|---|
| test/endpoint_unittest.cpp | Adds regression test asserting IPv6 endpoint2hostname() prints port in host byte order. |
| src/butil/details/extended_endpoint.hpp | Fixes IPv6 hostname formatting to use host-order port. |
Review effort: Lite
|
|
||
| std::string host_str; | ||
| ASSERT_EQ(0, butil::endpoint2hostname(ep, &host_str)); | ||
| ASSERT_EQ(host, host_str); |
| size_t len = ::strlen(host); | ||
| if (len + 1 < host_len) { | ||
| snprintf(host + len, host_len - len, ":%d", _u.in6.sin6_port); | ||
| snprintf(host + len, host_len - len, ":%d", ntohs(_u.in6.sin6_port)); |
Copilot stopped reviewing on behalf of
chenBright due to an error
November 11, 2026 12:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: resolve #3734
Problem Summary:
butil::endpoint2hostname()of an IPv6 endpoint prints the port in network byte order.[::1]:8080givesip6-localhost:36895on little-endian machines, whileendpoint2str()and the IPv4 path print 8080.ExtendedEndPoint::to_hostname()formats_u.in6.sin6_portwithoutntohs();ExtendedEndPoint::to(EndPointStr*)already converts it. Inside bRPC the wrong port shows on/connectionsfor IPv6 peers when-show_hostname_instead_of_ipis on.What is changed and the side effects?
Changed:
ExtendedEndPoint::to_hostname()printsntohs(_u.in6.sin6_port).EndPointTest.ipv6_endpoint2hostname_port. It fails on master (ip6-localhost:36895) and passes with this change. It is skipped when::1has no reverse name on the machine.test_butil --gtest_filter='EndPointTest.*'passes (23 tests) with gcc 13.3, CMake build, on Linux x86_64. The rest oftest_butil, the make and bazel builds, clang and macOS were not run locally.ntohs()call insrc/butil/details/extended_endpoint.hppand the new test intest/endpoint_unittest.cpp. The commit carriesGenerated-by:as the ASF generative tooling guidance recommends.Side effects:
Performance effects: none.
Breaking backward compatibility: none, apart from the port now being the real one.
Check List:
🤖 Generated with Claude Code